fix(runtime): classify usage-limit failures behind auth statuses as billing - #3660
Conversation
…illing Fixes apache#2516. Providers report exhausted plan windows, credits, and subscriptions through different evidence channels: some use explicit structured codes (OpenAI insufficient_quota arrives even as 429; DeepSeek sends insufficient_balance), and some gate the window behind credential-shaped 401/403 statuses for validly signed-in users, which the status-first fallback projected to 'Authentication failed' — pointing the user at re-authenticating when the useful action is waiting for the window to reset or checking the subscription. - New PROVIDER_BILLING_PROVIDER_CODES set checked with the other structured-code sets, before every numeric HTTP fallback: explicit provider evidence outranks the bare status (the same precedence the capacity and overflow sets already follow). - The 401/403 fallback now consults USAGE_LIMIT_TEXT_PATTERNS over the composite text: quota/usage-limit/plan/credit-exhaustion wording projects to ProviderBilling instead of Auth. Plain invalid-key and permission messages carry none of that vocabulary and stay Auth. - ProviderBilling already maps to a non-retryable policy in providerRetryMetadata, so closed plan windows stop being retried blindly while transient throttles keep their RateLimit path. Tests: classifier matrix for structured codes across 401/403/429, plan-window wording via both SDK carriers (error message and raw response body after a schema-parse failure), and non-regression pins for genuine invalid-key and permission failures. Signed-off-by: Yunare Maia <yunare@gmail.com>
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
#3660 f9b0dd8 — review (bind exact head)
Gate: CI test success on this head (run 32682534889, check_runs 1/1 success after approval). Fork PR previously blocked on action_required.
Verdict: GO (no P0-P2)
Classification narrowly fixes usage-limit behind 401/403: structured billing codes outrank status, and 401/403 fallback consults usage-limit text patterns over composite text, preserving genuine Auth. Non-retryable billing via providerRetryMetadata correct. Tests cover matrix.
Coexistence note: #2521 overlaps same defect file; #3660 is minimal fix shape. Epoch handling for any follow-up: rebase to current main and take strictly greater than base (do not hardcode number).
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Approving at f9b0dd835713.
Gate at this exact head: hosted test is terminal green — note that this PR's checks had never run at all until the fork workflow was released (32682534889), so the earlier empty check-runs was "not run", not "passed". Zero review threads, no APPROVED review bound to any older commit, and the branch is mergeable.
The change stays inside the file that owns the defect: packages/runtime/src/provider-error-classification.ts and its test, +109/-1, classifying usage-limit failures behind the auth status rather than collapsing them into a generic auth error.
Worth recording for whoever handles this next: #2521 fixes the same defect (both branches are named for issue 2516, and both edit this same file and test), but does it across 46 files and +4428/-173. That one is separately marked NO-GO. If this lands first, #2521 will need to be rebased and reduced to whatever remains genuinely unaddressed.
Approval only; merging is a human's call.
|
LGTM — merged. Thanks for the contribution! 中文已合并，感谢贡献。 |
2 participants
Summary
Fixes #2516.
Providers report exhausted plan windows, credits, and subscriptions through different evidence channels, and the status-first fallback mislabeled all of them:
insufficient_quotaeven arrives as 429, DeepSeek sendsinsufficient_balance.Auth→ "Authentication failed" — pointing the user at re-authenticating when the useful action is waiting for the window to reset or checking the subscription.Changes in
packages/runtime/src/provider-error-classification.ts, at the shared classifier boundary (no provider-specific branches, no user-facing strings in adapters):PROVIDER_BILLING_PROVIDER_CODESset, checked with the other structured-code sets before every numeric HTTP fallback — explicit provider evidence outranks the bare status, the same precedence the capacity and overflow sets already follow. This also stops an exhausted quota arriving as 429 from being classified as a transient throttle.401/403fallback now consultsUSAGE_LIMIT_TEXT_PATTERNSover the composite text: quota / usage-limit / plan / credit-exhaustion wording projects toProviderBillinginstead ofAuth. Plain invalid-key and permission messages carry none of that vocabulary and stayAuth.ProviderBillingalready maps to a non-retryable policy inproviderRetryMetadata, so closed plan windows stop being retried blindly while transient throttles keep theirRateLimitpath.Verification
Local run this time (
npm ciunblocked by pointing the six Azure DevOps mirror URLs inpackage-lock.jsonat their identical public npm packages; lockfile restored before commit):npm run build -w @maka/core && npm run build -w @maka/storage && npm run build -w @maka/mcp && npm run build -w @maka/runtime— all passnode --test dist/__tests__/provider-error-classification.test.js— 15 tests, 15 pass, 0 failAuth≠ProviderBilling) while the non-regression pins keep passing, then green again with the fix restoredNew tests cover: structured usage-limit codes across 401/403/429 (+ non-retryable metadata), plan-window wording through both SDK carriers (error message and raw response body after a schema-parse failure), and genuine invalid-key / permission failures staying
Auth.One scope note: 429 responses whose text suggests a long cap but that carry no structured code still classify as
RateLimit— distinguishing those would need per-provider cap wording I can't verify offline, and guessing risked false billing positives on real throttles.